Conversation
82d256e to
db8f98f
Compare
|
doc-kit supports generics, but only basic ones at the moment. Most of these are yes to be supported. You'll need to use syntax like |
plugins/theme/partials/types.mjs
Outdated
| return resolve(type.elementType) + '?'; | ||
|
|
There was a problem hiding this comment.
When a type is optional, we should just return the type, there's other ways to say it's optional in doc-kit
There was a problem hiding this comment.
Done, reverted to just returning the type.
plugins/theme/partials/types.mjs
Outdated
| return union(type.elements); | ||
| return `[${(type.elements ?? []).map(resolve).join(', ')}]`; |
There was a problem hiding this comment.
Updated to Tuple<>.
plugins/theme/partials/types.mjs
Outdated
| case 'conditional': | ||
| return `${resolve(type.trueType)}|${resolve(type.falseType)}`; | ||
| return `${resolve(type.checkType)} extends ${resolve(type.extendsType)} ? ${resolve(type.trueType)} : ${resolve(type.falseType)}`; |
There was a problem hiding this comment.
We don't support this yet, please revert
plugins/theme/partials/types.mjs
Outdated
|
|
||
| case 'indexedAccess': | ||
| return resolve(type.elementType ?? type.objectType); | ||
| return `${resolve(type.objectType)}[${resolve(type.indexType)}]`; |
|
Just because generics render a bit strange in the markdown, doesn't mean they'll render strange in doc-kit, please keep the curly brace syntax |
d6f52e0 to
36c52b8
Compare
|
Updated the tuple case to use |
|
Reverted to curly brace syntax. |
plugins/theme/partials/types.mjs
Outdated
| return resolve(type.elementType); | ||
|
|
There was a problem hiding this comment.
| return resolve(type.elementType); |
There was a problem hiding this comment.
Done, merged with indexedAccess.
| case 'reference': | ||
| return type.name; | ||
|
|
||
| case 'reference': { |
There was a problem hiding this comment.
We can merge this with the above case
There was a problem hiding this comment.
Done, merged intrinsic and reference into one case.
plugins/theme/partials/types.mjs
Outdated
|
|
||
| case 'tuple': | ||
| return union(type.elements); | ||
| return `Tuple<${(type.elements ?? []).map(resolve).join(', ')}>`; |
There was a problem hiding this comment.
nit: let's make union(type.elements, ', ') work
There was a problem hiding this comment.
Done, updated union to accept a separator param and used union(type.elements, ', ') for tuple.
|
I'll regenerate the docs to make the check pass when I get a chance |
36c52b8 to
3c80ad6
Compare
|
Thanks! |
Co-Authored-By: Aviv Keller <me@aviv.sh>
3c80ad6 to
3b33ac8
Compare
Summary
Fixes #30
The custom type resolver in
plugins/theme/partials/types.mjswas collapsing TypeScript types too aggressively, producing semantically inaccurate documentation output. This PR restores the correct semantics now that@node-core/doc-kitsupports generic types.Changes
referencetypes now include generic type arguments{Map},{ReadonlySet},{Record}{Map<string, number>},{ReadonlySet<string>},{Record<string, string|string[]|T>}tupletypes now render as tuple syntax instead of union syntax{number|number|number}{[number, number, number]}conditionaltypes now preserve the full conditional structure{TrueType|FalseType}{CheckType extends ExtendsType ? TrueType : FalseType}optionaltuple members now render with a trailing?suffixindexedAccesstypes now render asT[K]instead of justTVerification
After regenerating docs with
npm run generate-docs, the affected files show correct output:pages/v5.x/webpack/namespaces/esm.md:{Map<string, number>},{ReadonlySet<string>},{Iterable<Chunk>}pages/v5.x/webpack/namespaces/sharing.md:{Record<string, string|string[]|T>}pages/v5.x/webpack/namespaces/dependencies.md:{[number, number]}instead of{number|number}The regenerated
pages/v5.x/output is included in this PR.